runtimetest: add masked paths validation #82
Conversation
| return err | ||
| } | ||
| if fi.Mode()&0444 != 0 { | ||
| return fmt.Errorf("%v should be readonly", v) |
There was a problem hiding this comment.
The error message shouldn't be about “readonly”. And this is probably another place where we want to use “just try it” (in this case, read and write) instead of a mode check.
There was a problem hiding this comment.
@wking spec says "mask over the provided paths inside the container so that they cannot be read." it definitely says change paths permission to make them can't be read.
So, I think there is no need to "just try it".
There was a problem hiding this comment.
On Fri, May 27, 2016 at 12:30:03AM -0700, Ma Shimiao wrote:
spec says "mask over the provided paths inside the container so that
they cannot be read." it definitely says change paths permission to
make them can't be read.
I don't read “mask over” as clearly “change the permission bits”. I
haven't been able to turn up runC's implementation with grep, but
@mrunalp's original runtime-spec PR was clearly recommending
bind-mounts 1.
There was a problem hiding this comment.
Ah, maybe we need "just try it". I get from runc code, it implements maskedpaths as bing mount them to /dev/null
It makes a bit hard to validate.
Mashimiao
force-pushed
the
runtime-test-maksed-path-validation
branch
from
25c9c96
to
b891e5e
Compare
|
We can add a test that tries to read from a path and gets a EOF. f, err := os.Open(maskedPath)
if err != nil {
return err
}
defer f.Close()
b := make([]byte, 1)
_, err = f.Read(b)
if err == io.EOF {
fmt.Println("Test succeeded")
} else {
fmt.Println("Test failed")
} |
Mashimiao
force-pushed
the
runtime-test-maksed-path-validation
branch
from
b891e5e
to
74a274e
Compare
|
@mrunalp I combined your suggested code with mine. If a maskedPath can not be read, an error "permission denied" will occur, I think that is not what we want. |
|
@Mashimiao Are you seeing a permission denied with runc? I think it shouldn't return that error in any scenario. |
|
@mrunalp It seems I think too much :-). runc should be run by root. |
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Mashimiao
force-pushed
the
runtime-test-maksed-path-validation
branch
from
74a274e
to
615585a
Compare
|
LGTM |
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com